-
-
Notifications
You must be signed in to change notification settings - Fork 256
New options flag to output ES2015 modules #340
Conversation
PR for issue #338 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, we should improve supporting esmodules for css-loader, maybe for html-loader, just try to add tests for css-loader and you can see problem, anyway feel free to send a PR to css-loader
|
Just checked |
@joseprio no, look here https://github.com/webpack-contrib/css-loader/blob/master/src/runtime/getUrl.js#L1, now it is not string, it is object with |
Hi @evilebottnawi, |
@jpreynat Why do you need this? In webpack@5 we have |
@evilebottnawi Thanks for the info, I didn't see this new experiment on webpack 5. Also, it seems to lack configuration. For instance, with That's a lot of things missing for us with the current state of the |
// TODO: (hiroppy) use ESM will be done in near future
Can you clarify?
You can use Look in
Which? The file-loader will be deprecated after webpack@5 was released. |
@evilebottnawi Great if switching to ESM can be done quickly. Finally, we were using the |
Yes, we will do it, but i can't understand why you need modules here? It is always will be interpreted as default (you can't use named import for assets 😄 )
In Todo, but why don't use null-loader for this when you build node project? |
@evilebottnawi Using ES modules should allow webpack to further concatenate our bundle by using module concatenation for our assets too. |
Please clarify? Do you mean include url to asset without extra module, right? |
@evilebottnawi |
@jpreynat |
@joseprio |
@joseprio |
@jpreynat okay, let's merge it and released, also i will add tests for this cases for asset module to ensure all works fine in webpack@5 Can you answer about emitFile? |
We're just using `patch-package`; not ideal from a maintenance point of
view, but does the trick.
…On Wed, 20 Nov 2019 at 14:24, Johan Preynat ***@***.***> wrote:
@joseprio <https://github.com/joseprio>
Could you share any insights on how you managed to tweak webpack in order
to achieve this on your side? Was it simply to work with a fork of
file-loader or did you make other/further optimizations?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#340?email_source=notifications&email_token=AAMTJU2I6MWJN2H3T3P6KQ3QUVCBPA5CNFSM4IFSYDSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEESEJWY#issuecomment-556025051>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMTJU76NNS64V27QVPKK4DQUVCBPANCNFSM4IFSYDSA>
.
|
@evilebottnawi Regarding the @joseprio |
Maybe not bad idea |
Just a suggestion, but that would allow to fine-grain process the assets. |
Codecov Report
@@ Coverage Diff @@
## master #340 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 2 2
Lines 28 29 +1
Branches 11 13 +2
=====================================
+ Hits 28 29 +1
Continue to review full report at Codecov.
|
just testing, work fine with |
Thanks for the release @evilebottnawi |
This PR contains a:
Motivation / Use-Case
Using the old CommonJS module syntax breaks some optimizations that Webpack does in newer versions, like tree shaking and module concatenation. This flags allows the output to be ES2015 modules if enabled.
Breaking Changes
Default is
false
, so it shouldn't break anything unless used.Additional Info